Hi @YDHCUI ! Thanks for the contribution.

Few things we will need to address before merge:

1. cargo clippy and cargo fmt need to pass

2. Performance Regression: Boxing Streams

Files: src/client.rs:213-217

Before:

read_stream: tokio::net::tcp::OwnedReadHalf,
write_stream: Arc<tokio::sync::Mutex<tokio::net::tcp::OwnedWriteHalf>>,

After:

read_stream: Box<dyn AsyncRead + Unpin + Send + Sync>,
write_stream: Arc<tokio::sync::Mutex<Box<dyn AsyncWrite + Unpin + Send + Sync>>>,

Issue: Every read/write operation now goes through:

1. Heap indirection (Box)
2. Dynamic dispatch (vtable lookup)

For a VNC server performing thousands of small I/O operations per second, this adds measurable overhead. The original concrete types allowed the compiler to inline and optimize I/O calls.

3. Performance Regression: tokio::io::split() vs into_split()

File: src/client.rs:392

Before:

let (read_stream, write_stream) = stream.into_split();

After:

let (read_stream, write_stream) = tokio::io::split(stream);

Issue: TcpStream::into_split() is zero-cost - it returns owned halves with no synchronization overhead. tokio::io::split() uses an internal Arc<Mutex<>> to coordinate access between the read and write halves.

This means every I/O operation on generic streams now acquires a mutex lock, even when reads and writes don't overlap.

4. Nagle's Algorithm Latency

File: src/client.rs:327

Before:

stream.set_nodelay(true)?;

After:

//stream.set_nodelay(true)?;

Issue: The call to disable Nagle's algorithm is commented out entirely. For TCP connections, this means small packets will be delayed (up to 200ms on some systems) waiting for more data to coalesce.

VNC sends many small messages (mouse movements, key events, small screen updates). With Nagle enabled, these will be delayed, causing noticeable input lag.

Better fix: Check if the stream is a TcpStream and call set_nodelay() conditionally, or add a parameter to control this behavior.

Severity: High - Significant latency regression for TCP connections.

5. Loss of Remote Host Information

File: src/client.rs:324-325

Before:

let remote_host = stream
    .peer_addr()
    .map_or_else(|_| "unknown".to_string(), |addr| addr.to_string());

After:

let remote_host = None; // Generic streams may not have peer_addr

Issue: The remote host is now always None for all connections, including TCP. This loses valuable diagnostic information for:

• Logging
• Connection debugging
• Security auditing
• Rate limiting by IP

Better fix: Accept Option<String> as a parameter to new() or from_socket() so callers can provide the remote address when known.

6. Server-side Stream Type Change

File: src/server.rs:64-65

Before:

client_write_streams:
    Arc<RwLock<Vec<Arc<tokio::sync::Mutex<tokio::net::tcp::OwnedWriteHalf>>>>>,

After:

client_write_streams:
    Arc<RwLock<Vec<Arc<tokio::sync::Mutex<Box<dyn tokio::io::AsyncWrite + Unpin + Send + Sync>>>>>>,

Issue: Same boxing overhead as issue #1. All write stream handles are now boxed trait objects with dynamic dispatch.

7. Unsafe Send Implementation

File: examples/generic_stream.rs:248-249

// Implement Send since S is Send
unsafe impl<S: Send> Send for ExampleStreamWrapper<S> {}

Issue: Using unsafe impl Send is dangerous and rarely necessary. If S is Send and all fields are Send, the struct should automatically be Send. The explicit unsafe impl suggests the struct contains non-Send types (dangerous to override). However, it doesn't seem that it actually does.

In this case, ExampleStreamWrapper only contains S (which is Send) and usize (which is Send), so the unsafe impl is unnecessary.

// These two lines can both be deleted:

impl<S: Unpin> Unpin for ExampleStreamWrapper<S> {}                                                                                                                                                       
unsafe impl<S: Send> Send for ExampleStreamWrapper<S> {}                                                                                                                                                  

Rust will automatically derive both traits since S and usize implement them.

Severity: Low - Unnecessary but harmless in this case.

Recommendations

1. Keep TCP-optimized path: Don't change the core types. Add from_socket() as an alternative that uses generic types only when needed.

2. Conditional set_nodelay(): Use trait bounds or runtime type checking to call set_nodelay() for TCP streams:

   if let Some(tcp) = stream.downcast_ref::<TcpStream>() {
       tcp.set_nodelay(true)?;
   }

3. Pass remote address as parameter: Add remote_addr: Option<String> parameter to from_socket().